Update ext/oci8 methodsynopses based on stubs #601

kocsismate · 2021-05-13T22:13:48Z

No description provided.

Girgias

Looks mostly good to me other than a couple of changes, but I haven't really followed all the stub changes discussions.

reference/oci8/OCI-Lob/close.xml

reference/oci8/functions/oci-bind-array-by-name.xml

reference/oci8/functions/oci-bind-by-name.xml

reference/oci8/functions/oci-close.xml

reference/oci8/functions/oci-fetch-object.xml

reference/oci8/functions/oci-free-descriptor.xml

reference/oci8/functions/oci-new-descriptor.xml

reference/oci8/functions/oci-password-change.xml

kocsismate · 2021-07-18T14:30:00Z

@Girgias Can you please take another look?

cmb69

From a cursory look, this seems to be okay. I don't have any experience with OCI, and I don't have the time to check PHP 7 implementation regarding potential changes. Maybe @cjbj wants to have look?

cjbj · 2021-08-20T07:13:46Z

Gotta love the PHP community for being active on this! Thanks all.

Visually it seemed OK but my long-working build command php ../doc-base/configure.php && phd -o mydocs -f xhtml -P PHP -p book.oci8 -d ../doc-base/.manual.xml failed with

ERROR (/Users/cjones/p/phpdoc/doc-base/manual.xml:487:0)
End:
^
 XPointer evaluation failed: #xmlns(db=http://docbook.org/ns/docbook)
 xpointer(id('ingres.configuration.list')/*)

ERROR (/Users/cjones/p/phpdoc/doc-base/manual.xml:unknown)
 XPointer evaluation failed: #xmlns(db=http://docbook.org/ns/docbook)
 xpointer(id('mongo.configuration.list')/*)
failed.

The document didn't validate, here are the errors I got:
(If this isn't enough information, try again with --enable-xml-details)
 IDREF attribute linkend references an unknown ID "book.filepro"
 IDREF attribute linkend references an unknown ID "book.filepro"
 IDREF attribute linkend references an unknown ID "book.filepro"
[... and a bunch more]

Hints anyone?

cmb69 · 2021-08-20T07:53:18Z

@cjbj, I think you need to rebase this PR onto HEAD of master (also update doc-base and phd if not already done).

cjbj

@cjbj, I think you need to rebase this PR onto HEAD of master (also update doc-base and phd if not already done).

Good call.

reference/oci8/OCI-Collection/getElem.xml

reference/oci8/OCI-Collection/append.xml

cjbj · 2021-08-25T07:48:55Z

reference/oci8/OCI-Collection/assign.xml

@@ -9,7 +9,7 @@
 <refsect1 role="description">
  &reftitle.description;
  <methodsynopsis role="oop">
-   <type>bool</type><methodname>OCICollection::assign</methodname>
+   <modifier>public</modifier> <type>bool</type><methodname>OCICollection::assign</methodname>


It seems OCICollection::assignElem wasn't updated.

Ohh, I think that must be because of the different capitalization (assignElem vs assignelem). :/

I created php/php-src#7405 to fix the inconsistency

That new PR doesn't seem to add the public modifier to OCICollection::assignElem ?

No, the new PR is unrelated from <modifier>public</modifier>. It is only added because the script always generates the access modifier for methods, while OCICollection::assign() didn't have one previously.

Yay for tidy-ups! Will you update this (601) PR with <modifier> for OCICollection::assignElem, Lob::truncate, getBuffering and setBuffering, and writeTemporary?

Yes, I've just pushed all the changes. In the meanwhile, I found out why OCILob::truncate() wasn't autogenerated: c3978de#diff-1ef7c5160d41789c5faf14de56219ac73b3039795ebbac6da6ebaa716cdd8d21L12 :D It's fixed now.

cjbj · 2021-08-25T07:51:31Z

reference/oci8/OCI-Lob/erase.xml

@@ -80,13 +86,12 @@
  &reftitle.seealso;
  <para>
   <simplelist>
-    <member><xref linkend="oci-lob.truncate" /></member> 
+    <member><xref linkend="oci-lob.truncate"/></member> 


It looks like the prototype for Lob::truncate itself hasn't been updated.

What do you miss from Lob::truncate()? It looks good for me.

cjbj · 2021-08-25T07:52:17Z

reference/oci8/OCI-Lob/flush.xml

-    <member><xref linkend="oci-lob.getbuffering" /></member>
-    <member><xref linkend="oci-lob.setbuffering" /></member>
+    <member><xref linkend="oci-lob.getbuffering"/></member>
+    <member><xref linkend="oci-lob.setbuffering"/></member>


The APIs for getBuffering and setBuffering don't seem to have been updated.

cjbj · 2021-08-25T07:52:48Z

reference/oci8/OCI-Lob/close.xml

  </methodsynopsis>
  <para>
   Closes descriptor of LOB or FILE. This function should be used only with
-   <xref linkend="oci-lob.writetemporary" />.
+   <xref linkend="oci-lob.writetemporary"/>.


The API for writeTemporary doesn't seem to have been updated.

cjbj · 2021-08-25T08:02:02Z

reference/oci8/functions/oci-fetch-all.xml

-   <methodparam choice="opt"><type>int</type><parameter>flags</parameter><initializer><constant>OCI_FETCHSTATEMENT_BY_COLUMN</constant> + <constant>OCI_ASSOC</constant></initializer></methodparam>
+   <methodparam choice="opt"><type>int</type><parameter>offset</parameter><initializer>0</initializer></methodparam>
+   <methodparam choice="opt"><type>int</type><parameter>limit</parameter><initializer>-1</initializer></methodparam>
+   <methodparam choice="opt"><type>int</type><parameter>flags</parameter><initializer>0</initializer></methodparam>


I don't think showing the default as 0 is helpful to readers

Nice catch! I'll fix it in php-src as well

Submitted php/php-src#7429

kocsismate · 2021-09-14T07:39:38Z

Can I get a (hopefully) final review?

Girgias

Other than the minor nit in how the changelog is written (which well we're already inconsistent about so not that important), LGTM but then I don't know anything about OCI8

reference/oci8/OCI-Lob/erase.xml

reference/oci8/OCI-Lob/export.xml

reference/oci8/OCI-Lob/write.xml

reference/oci8/functions/oci-new-collection.xml

reference/oci8/functions/oci-lob-copy.xml

reference/oci8/functions/oci-error.xml

reference/oci8/functions/oci-connect.xml

reference/oci8/OCI-Lob/writeToFile.xml

reference/oci8/functions/oci-new-connect.xml

kocsismate · 2021-09-22T20:56:45Z

I rebased + added a new commit (964cbcd) which fixes a few minor things (+ an overloaded signature).

I'd love if I could merge this PR soon, after 4 months. :/

cjbj · 2021-09-23T08:04:57Z

@cmb69 @Girgias are you OK with this? Can it be merged?

reference/oci8/functions/oci-field-name.xml

reference/oci8/functions/oci-client-version.xml

reference/oci8/functions/oci-field-precision.xml

reference/oci8/functions/oci-field-scale.xml

reference/oci8/functions/oci-field-size.xml

reference/oci8/functions/oci-field-type-raw.xml

reference/oci8/functions/oci-field-type.xml

reference/oci8/functions/oci-num-rows.xml

reference/oci8/functions/oci-password-change.xml

kocsismate · 2021-09-23T12:57:15Z

Thank you, @Girgias !

cjbj · 2021-09-27T04:39:48Z

Thanks everyone!

kocsismate added the method synopsis label May 13, 2021

Girgias requested changes May 15, 2021

View reviewed changes

kocsismate requested a review from Girgias June 18, 2021 13:50

afilina added the Waiting on Review label Aug 17, 2021

afilina requested a review from cmb69 August 17, 2021 13:47

cmb69 reviewed Aug 17, 2021

View reviewed changes

cjbj requested changes Aug 25, 2021

View reviewed changes

Girgias requested changes Sep 14, 2021

View reviewed changes

kocsismate added 6 commits September 22, 2021 22:53

Update ext/oci8 methodsynopses based on stubs

ba2fb77

Address review comments

756c498

Add changelog entries for nullable parameters

5bf2bdd

Fix a few issues

610f3a7

Improve wording of changelog items

1cc65c4

A few description fixes

964cbcd

kocsismate force-pushed the oci8-stubs branch from fe73de6 to 964cbcd Compare September 22, 2021 20:54

Girgias approved these changes Sep 23, 2021

View reviewed changes

Girgias added 2 commits September 23, 2021 13:51

Use XML entities

b1a082a

Use XML entities

9b3a7bb

Girgias merged commit ed6de1a into php:master Sep 23, 2021

kocsismate deleted the oci8-stubs branch September 23, 2021 12:57

afilina removed the Waiting on Review label Dec 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update ext/oci8 methodsynopses based on stubs #601

Update ext/oci8 methodsynopses based on stubs #601

kocsismate commented May 13, 2021

Girgias left a comment

kocsismate commented Jul 18, 2021

cmb69 left a comment

cjbj commented Aug 20, 2021

cmb69 commented Aug 20, 2021

cjbj left a comment

cjbj Aug 25, 2021

kocsismate Aug 26, 2021

kocsismate Aug 26, 2021

cjbj Aug 30, 2021

kocsismate Aug 30, 2021 •

edited

Loading

cjbj Aug 31, 2021

kocsismate Aug 31, 2021

cjbj Aug 25, 2021

kocsismate Aug 30, 2021

cjbj Aug 25, 2021

cjbj Aug 25, 2021

cjbj Aug 25, 2021

kocsismate Aug 30, 2021

kocsismate Aug 30, 2021

kocsismate commented Sep 14, 2021

Girgias left a comment

kocsismate commented Sep 22, 2021

cjbj commented Sep 23, 2021

kocsismate commented Sep 23, 2021

cjbj commented Sep 27, 2021

Update ext/oci8 methodsynopses based on stubs #601

Update ext/oci8 methodsynopses based on stubs #601

Conversation

kocsismate commented May 13, 2021

Girgias left a comment

Choose a reason for hiding this comment

kocsismate commented Jul 18, 2021

cmb69 left a comment

Choose a reason for hiding this comment

cjbj commented Aug 20, 2021

cmb69 commented Aug 20, 2021

cjbj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kocsismate Aug 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kocsismate commented Sep 14, 2021

Girgias left a comment

Choose a reason for hiding this comment

kocsismate commented Sep 22, 2021

cjbj commented Sep 23, 2021

kocsismate commented Sep 23, 2021

cjbj commented Sep 27, 2021

kocsismate Aug 30, 2021 •

edited

Loading